Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SeaLabs Build Above Water #3982

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

robertthepie
Copy link
Contributor

@robertthepie robertthepie commented Nov 28, 2024

Work done

Moves the Sea Lab build point before construction starts so that subs are build floating instead of underwater incase the water is too shalow, as during this state they are not testing debth
*i couldn't figure out how to get the build piece so the list is unfortunately hardcoded for now

Addresses Issue(s)

Setup

Place Sea labs corsy, corasy, armsy, armasy, and build subs with them as wella s non subs

Test steps

  • Place the above mentioned labs, and build subs and boats both interchangably and sequencially to make sure a unit doesn't randomly get placed at another's debth goal

Screenshots:

BEFORE:

image

AFTER:

image

@WatchTheFort
Copy link
Member

If the water is too shallow for the subs, then it need to be too shallow for the shipyard too. A shipyard should only be placeable in water deep enough to build every unit in the build list.

@robertthepie
Copy link
Contributor Author

If the water is too shallow for the subs, then it need to be too shallow for the shipyard too. A shipyard should only be placeable in water deep enough to build every unit in the build list.

correcl has a water debth of 16, the problem is that is has a waterline of 80, if kept to the latter number sea labs would be unusable on many maps instead.

@SethDGamre i can't pinpoint the issue with corasy cortex t2 sea lab
the corsy, armsy, and armasy all work fine when building subs

@robertthepie
Copy link
Contributor Author

okay, this turns out to be a much weirder problem than imagined.
If you put a shipyard on land it will build the sub on land
If you put it in deep enough, but not deep enough for the unit to reach its debth water it will build it along the sea floor at best
But at the right shallow debth it is a free for all, that thing is now underground

@robertthepie robertthepie marked this pull request as draft November 28, 2024 22:25
local subDefIDs = {}
do
local shipyardsNamesToPads = {
corsy = 4,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put piece name as a customparam imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally wanted to call the QueryBuildInfo but unfortunately:
1: I couldn't figure out how to get the call to work to get the return param back, if that is even possible.
2: Knowing some of our code malpractices around (me), this could potentially backfire one day.
Will move it to a custom param once i figure out the cases under which the unit goes underground.

Copy link
Collaborator

@sprunk sprunk Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. requires LUS afaik, but factories are a great porting target anyway (their script code is simple and they are rare enough for perf not to be a concern). Calling QueryBuildInfo here in the gadget would be a bad idea though because the piece can change and then you'd leave the previous one at a different height, but when you have LUS you can just put the logic in each shipyard's script
  2. stuff blowing up is good, how else are yall gonna learn?

end

for unitDefID, unitDef in pairs(UnitDefs) do
if unitDef.waterline and unitDef.minWaterDepth and unitDef.waterline >= 30 then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could check movedef, it has an explicit submarine bool

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from what i can find, unitDef.moveDef.subMarine? or would it be moveDef.smClass?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isSubmarine, also make sure not to index the movedef without a nil check because immobiles/aircraft don't have one

local md = unitDef.moveDef
if md and md.isSubmarine then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants